Improve Message Box in WinForms #13440

memoarfaa · 2025-05-11T16:28:18Z

Fixes #12044

Proposed changes

Style, localize Message Box and add some public API to use custom icon.

Screenshots

Before

After

Test methodology

Manually tested via scratch project.

Microsoft Reviewers: Open in CodeFlow

codecov · 2025-05-11T17:03:50Z

Codecov Report

Attention: Patch coverage is 0% with 141 lines in your changes missing coverage. Please review.

Project coverage is 76.58368%. Comparing base (72ee6eb) to head (925fc08).
Report is 152 commits behind head on main.

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main      #13440         +/-   ##
===================================================
+ Coverage   75.43847%   76.58368%   +1.14521%     
===================================================
  Files           3230        3230                 
  Lines         639097      639238        +141     
  Branches       47289       47308         +19     
===================================================
+ Hits          482125      489552       +7427     
+ Misses        147952      146112       -1840     
+ Partials        9020        3574       -5446

Flag	Coverage Δ
Debug	`76.58368% <0.00000%> (+1.14521%)`	⬆️
integration	`18.79525% <0.00000%> (?)`
production	`50.97896% <0.00000%> (+2.56687%)`	⬆️
test	`97.40411% <ø> (ø)`
unit	`48.37502% <0.00000%> (-0.03707%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

src/System.Windows.Forms/System/Windows/Forms/Dialogs/MessageBox.cs

…hen DarkMode is enabeld

KlausLoeffelmann · 2025-05-12T19:35:08Z

Hey guys,

so, I am a bit confused.
In the discussion to the issue, the sentiment seemed to be not taking any changes for the old MessageBox, let alone add new APIs to it.

So, it's not exactly clear to me, what we want to do here.
We cannot change the API of the existing MessageBox, as it would break the world, almost literally.

But we will also not have an additional MessageBox, which would do essentially do the same as the old one. We said that we want the TaskDialog to move forward, and I am looking actively at what options we might have, to have that one adapted to dark mode.

Yes, it's not ideal that we do not have dark mode support for the native MessageBox-es as it is.
And even if we did not touch their APIs - looking at the code, I am not entirely sure, if we want to take the risk of hijacking too many of the API calls (but @JeremyKuhne would have the final saying in this, anyway) as a principal approach.

The only way to get the old MessageBox display acceptable in dark mode would probably be, to completely reimplement it in WinForms based on what we're actually supporting in terms of dark mode support.

That's something I had considered, and I have actually a prototype for testing, and would bring TO a team meeting if time allows during the Preview 6-time frame. But even that is not really likely to make the bar.

We need to discuss internally at the next team meeting, and I will follow up, but I do not see a good chance this being taken.

Klaus

AraHaan · 2025-05-12T20:48:14Z

Hey guys,

so, I am a bit confused. In the discussion to the issue, the sentiment seemed to be not taking any changes for the old MessageBox, let alone add new APIs to it.

So, it's not exactly clear to me, what we want to do here. We cannot change the API of the existing MessageBox, as it would break the world, almost literally.

But we will also not have an additional MessageBox, which would do essentially do the same as the old one. We said that we want the TaskDialog to move forward, and I am looking actively at what options we might have, to have that one adapted to dark mode.

Yes, it's not ideal that we do not have dark mode support for the native MessageBox-es as it is. And even if we did not touch their APIs - looking at the code, I am not entirely sure, if we want to take the risk of hijacking too many of the API calls (but @JeremyKuhne would have the final saying in this, anyway) as a principal approach.

The only way to get the old MessageBox display acceptable in dark mode would probably be, to completely reimplement it in WinForms based on what we're actually supporting in terms of dark mode support.

That's something I had considered, and I have actually a prototype for testing, and would bring TO a team meeting if time allows during the Preview 6-time frame. But even that is not really likely to make the bar.

We need to discuss internally at the next team meeting, and I will follow up, but I do not see a good chance this being taken.

Klaus

I think installing a hook and uninstalling the hook inside of ShowCore is fine, but allowing one to be able to:

customize the colors beyond just going into dark mode.
customize the icon.

Are both beyond the scope of MessageBox.

However, hooking into it's WndProc to override it into dark mode should be alright as long as the person who opened this PR makes it to where that is not part of the public API surface (to ensure existing code does not break) while also allowing dark mode out-of-box when the developer enables dark mode in their application. The reason for this is that I see a way that this can be done as an implementation detail without exposing any new public API's.

AraHaan

A few changes I feel can be done. Other than that LGTM. Alternatively the hook step for MessageBox could be split into a separate MessageBox.WndHook.cs file and the hook part from ShowCore into a new method named HookMessageBoxPInvoke basically calling PInvoke.MessageBox right after it hooks the messagebox and inside of it's own try block, and then in the finally block have it unhook it. And then in ShowCore have it call HookMessageBoxPInvoke with no need to cast the result to DialogResult as the HookMessageBoxPInvoke method would cast and return the DialogResult directly.

A benefit to this is then the hook stuff would go into it's own individual file for easier maintainability, but at the cost of making the MessageBox class partial.

AraHaan · 2025-07-02T05:31:59Z

src/System.Windows.Forms/System/Windows/Forms/Dialogs/MessageBox.cs

+#pragma warning disable WFO5001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
+            if (Application.IsDarkModeEnabled)
+            {
+                InstallHook();


Alternatively InstallHook could check if if (!Application.IsDarkModeEnabled) and instantly return if dark mode is not enabled to have minimal changes to this method. Same can be done inside of UninstallHook as well.

AraHaan · 2025-07-02T05:34:26Z

src/System.Windows.Forms/System/Windows/Forms/Dialogs/MessageBox.cs

+
+    private static void UninstallHook()
+    {
+        lock (s_lock)


As said below add in this:

Suggested change

lock (s_lock)

if (!Application.IsDarkModeEnabled)

{

return;

}

lock (s_lock)

AraHaan · 2025-07-02T05:35:23Z

src/System.Windows.Forms/System/Windows/Forms/Dialogs/MessageBox.cs

+
+    private static unsafe void InstallHook()
+    {
+        lock (s_lock)


As said below add in this:

Suggested change

lock (s_lock)

if (!Application.IsDarkModeEnabled)

{

return;

}

lock (s_lock)

AraHaan · 2025-07-02T05:36:48Z

src/System.Windows.Forms/System/Windows/Forms/Dialogs/MessageBox.cs

+            if (Application.IsDarkModeEnabled)
+            {
+                InstallHook();
+            }


Suggested change

if (Application.IsDarkModeEnabled)

{

InstallHook();

}

InstallHook();

AraHaan · 2025-07-02T05:37:10Z

src/System.Windows.Forms/System/Windows/Forms/Dialogs/MessageBox.cs

+            if (Application.IsDarkModeEnabled)
+            {
+                UninstallHook();
+            }


Suggested change

if (Application.IsDarkModeEnabled)

{

UninstallHook();

}

UninstallHook();

AraHaan · 2025-07-02T22:17:31Z

src/System.Windows.Forms.Primitives/src/NativeMethods.txt

@@ -149,6 +150,7 @@ GetCurrentThread
 GetCursor
 GetCursorPos
 GetDIBits
+GetDlgCtrlID


Suggested change

GetDlgCtrlID

This can be removed as it looks like it is not used.

memoarfaa added 2 commits May 11, 2025 08:58

Add MessageBox API Suggestion

f22357d

Complete MessageBox API Suggestion

dc9b4a9

memoarfaa requested a review from a team as a code owner May 11, 2025 16:28

github-actions bot added the area-Infrastructure label May 11, 2025

dotnet-policy-service bot assigned memoarfaa May 11, 2025

AraHaan reviewed May 12, 2025

View reviewed changes

src/System.Windows.Forms/System/Windows/Forms/Dialogs/MessageBox.cs Outdated Show resolved Hide resolved

AraHaan suggested changes May 12, 2025

View reviewed changes

dotnet-policy-service bot added the waiting-author-feedback The team requires more information from the author label May 12, 2025

AraHaan reviewed May 12, 2025

View reviewed changes

Remove all MessageBox improvment and add only dark MessageBox style w…

eabcaee

…hen DarkMode is enabeld

dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label May 12, 2025

memoarfaa added 2 commits May 12, 2025 17:14

remove PublicAPI.Unshipped and fix build error

81b583d

remove PublicAPI.Unshipped and fix build error

925fc08

Tanya-Solyanik added area-DarkMode Issues relating to Dark Mode feature and removed area-Infrastructure labels May 12, 2025

KlausLoeffelmann added the untriaged The team needs to look at this issue in the next triage label May 12, 2025

AraHaan suggested changes Jul 2, 2025

View reviewed changes

dotnet-policy-service bot added the waiting-author-feedback The team requires more information from the author label Jul 2, 2025

AraHaan reviewed Jul 2, 2025

View reviewed changes

dotnet-policy-service bot added 💤 no-recent-activity and removed 💤 no-recent-activity labels Jul 17, 2025

Improve Message Box in WinForms #13440

Are you sure you want to change the base?

Improve Message Box in WinForms #13440

Uh oh!

Conversation

memoarfaa commented May 11, 2025 • edited by dotnet-policy-service bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Screenshots

Before

After

Test methodology

Microsoft Reviewers: Open in CodeFlow

Uh oh!

codecov bot commented May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KlausLoeffelmann commented May 12, 2025

Uh oh!

AraHaan commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AraHaan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AraHaan Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

AraHaan Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

AraHaan Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

AraHaan Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

AraHaan Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

AraHaan Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

memoarfaa commented May 11, 2025 •

edited by dotnet-policy-service bot

Loading

codecov bot commented May 11, 2025 •

edited

Loading

AraHaan commented May 12, 2025 •

edited

Loading

AraHaan left a comment •

edited

Loading